Skip to content

refactor(Support): rewrite class component to functional#8173

Merged
alexander-akait merged 5 commits intowebpack:mainfrom
sujalgoel:refactor/support-to-functional
Apr 30, 2026
Merged

refactor(Support): rewrite class component to functional#8173
alexander-akait merged 5 commits intowebpack:mainfrom
sujalgoel:refactor/support-to-functional

Conversation

@sujalgoel
Copy link
Copy Markdown
Contributor

Summary

Part of #8161. Converts Support from a class component to a functional component using hooks. Moves the supporters computation into useMemo to satisfy the react-hooks/purity lint rule. Also adds tests.

What kind of change does this PR introduce?

Refactor. No behavior or visual changes.

Did you add tests for your changes?

Does this PR introduce a breaking change?

No.

If relevant, what needs to be documented once your changes are merged or what have you already documented?

N/A

Use of AI

No.

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 6, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
webpack-js-org Ready Ready Preview, Comment Apr 30, 2026 1:03pm

Request Review

<h2>
Backers
</h2>
`;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add more tests and handle more cases, including, image not loaded and other stuff

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

working on it!

Comment thread src/components/Support/Support.jsx Outdated
}
}
return () => observer.disconnect();
}, []); // eslint-disable-line react-hooks/exhaustive-deps
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to rewrite it to do not have any eslint-disable-line, they are enabled to prevent errors/problems, here we have it

Comment thread src/components/Support/Support.jsx Outdated
);
}
if (typeof age === "number") {
// eslint-disable-next-line react-hooks/purity
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still have eslint-disable-next-line react-hooks/purity, can you look at this, we should avoid any eslint problems

@alexander-akait
Copy link
Copy Markdown
Member

@sujalgoel Latest Sponsors is broken, no images, please check it and add test cases

@sujalgoel
Copy link
Copy Markdown
Contributor Author

Looked into this. The Latest Sponsors filter keeps supporters whose firstDonation is within the last 14 days, evaluated against Date.now() at runtime. The _supporters.json file is regenerated by fetch:supporters at build time, so once a build sits unrebuilt for a while the rolling 14-day window can shrink.

Reproduced locally. With the JSON that ships in the latest preview build (Apr 22), no supporters fall in the window today. After running npm run fetch:supporters against the live OpenCollective API, 3 recent sponsors appear (Apr 16, Apr 17, Apr 26), and the section renders all 3 avatars correctly: SmallIcon shows first, then swaps to the avatar URL once IntersectionObserver fires. naturalWidth: 128 for every image, no errors.

Same behavior as the previous class component, no regression from the refactor. Triggering a rebuild on the preview should populate the section.

Added test coverage for the latest rank:

  • new Support.latest.test.jsx with a 12 recent + 1 stale fixture: verifies the limit of 10, exclusion of supporters older than 14 days, SmallIcon initial state, and avatar swap after the observer fires
  • two cases added to Support.test.jsx: maxAge/limit text in the description, and zero images when the supporter list is empty

@alexander-akait alexander-akait merged commit e38421f into webpack:main Apr 30, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants